Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(x/fswap): introduce new event(MakeSwapProposal) #1363

Merged
merged 7 commits into from
May 8, 2024

Conversation

jaeseung-bae
Copy link
Contributor

@jaeseung-bae jaeseung-bae commented May 8, 2024

Description

  • Introduce new event for MakeSwapProposal
    • Event includes Swap and DenomMetadata

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

Copy link

codecov bot commented May 8, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 69.54%. Comparing base (8ad5585) to head (3b15e93).

❗ Current head 3b15e93 differs from pull request most recent head 43bd287. Consider uploading reports for the commit 43bd287 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1363      +/-   ##
==========================================
- Coverage   69.54%   69.54%   -0.01%     
==========================================
  Files         672      672              
  Lines       56106    56111       +5     
==========================================
+ Hits        39018    39020       +2     
- Misses      14825    14826       +1     
- Partials     2263     2265       +2     
Files Coverage Δ
x/fswap/keeper/proposal.go 48.54% <33.33%> (-1.46%) ⬇️

... and 1 file with indirect coverage changes

@jaeseung-bae jaeseung-bae marked this pull request as ready for review May 8, 2024 10:13
@jaeseung-bae jaeseung-bae marked this pull request as draft May 8, 2024 10:18
@jaeseung-bae jaeseung-bae marked this pull request as ready for review May 8, 2024 10:31
Copy link
Collaborator

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, EventMakeSwap would be more flexible (please consider upcoming msg-based proposals). Then, the emission should reside in MakeSwap.

@jaeseung-bae jaeseung-bae requested review from 0Tech and 170210 May 8, 2024 11:56
0Tech
0Tech previously approved these changes May 8, 2024
@jaeseung-bae jaeseung-bae merged commit 5e2cd7f into Finschia:main May 8, 2024
72 of 73 checks passed
@jaeseung-bae jaeseung-bae deleted the feat/add-denommeta-event branch May 8, 2024 12:39
mergify bot pushed a commit that referenced this pull request May 8, 2024
* feat(x/fswap): introduce new event(MakeSwapProposal)

* chore: lint proto

* chore: update proto-doc

* chore: update changelog

* chore: newline

* chore: update proto

* chore: make the event two separate events

(cherry picked from commit 5e2cd7f)
jaeseung-bae added a commit that referenced this pull request May 8, 2024
* feat(x/fswap): introduce new event(MakeSwapProposal)

* chore: lint proto

* chore: update proto-doc

* chore: update changelog

* chore: newline

* chore: update proto

* chore: make the event two separate events

(cherry picked from commit 5e2cd7f)

Co-authored-by: jaeseung-bae <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants